Support UDP multicast. #618

Lukasa · 2018-09-19T14:35:08Z

Motivation:

A large number of very useful protocols are implemented using multicast
with UDP. As a result, it would be helpful to add support for joining and
leaving IP multicast groups using SwiftNIO.

Modifications:

Defines a MulticastChannel protocol for channels that support joining and
leaving multicast groups.
Adds an implementation of MulticastChannel to DatagramChannel.
Adds a interfaceIndex property to NIONetworkInterface.
Adds if_nametoindex to the Posix enum.
Adds a isMulticast computed property to SocketAddress
Adds a demo multicast chat application.
Add a number of multicast-related socket options to SocketOptionProvider.

Result:

NIO users will be able to write channels that handle multicast UDP.

weissi

looks really good, didn't manage to go through everything but need to head to a meeting now

weissi · 2018-09-19T14:42:17Z

Sources/NIO/Channel.swift

@@ -337,6 +337,23 @@ public enum ChannelLifecycleError: Error {
    case inappropriateOperationForState
 }

+/// This should be inside of `ChannelError` but we keep it separate to not break API.
+// TODO: For 2.0: bring this inside of `ChannelError`.


mind filing an issue for that with the 2.0 milestone? Just so we won't forget and then we get nice github support of how many outstanding things

Sure, see #620.

weissi · 2018-09-19T14:42:46Z

Sources/NIO/Channel.swift

+
+    /// An attempt was made to join a multicast group that does not correspond to a multicast
+    /// address.
+    case notAMulticastAddress(SocketAddress)


maybe illegalMulticastAddress?

or invalidMulticastAddress

weissi · 2018-09-19T14:45:13Z

Sources/NIO/MulticastChannel.swift

+    /// - parameters:
+    ///     - group: The IP address corresponding to the relevant multicast group.
+    /// - returns: An `EventLoopFuture` that will be notified once the operation is complete.
+    func joinGroup(_ group: SocketAddress) -> EventLoopFuture<Void>


shouldn't this be only in the proto extension? There's only one good implementation for that

weissi · 2018-09-19T14:45:19Z

Sources/NIO/MulticastChannel.swift

+    ///     - group: The IP address corresponding to the relevant multicast group.
+    ///     - interface: The interface on which to join the given group, or `nil` to allow the kernel to choose.
+    /// - returns: An `EventLoopFuture` that will be notified once the operation is complete.
+    func joinGroup(_ group: SocketAddress, interface: NIONetworkInterface?) -> EventLoopFuture<Void>


shouldn't this be only in the proto extension? There's only one good implementation for that

weissi · 2018-09-19T14:45:25Z

Sources/NIO/MulticastChannel.swift

+    /// - parameters:
+    ///     - group: The IP address corresponding to the relevant multicast group.
+    /// - returns: An `EventLoopFuture` that will be notified once the operation is complete.
+    func leaveGroup(_ group: SocketAddress) -> EventLoopFuture<Void>


shouldn't this be only in the proto extension? There's only one good implementation for that

weissi · 2018-09-19T14:50:46Z

Sources/NIO/SocketChannel.swift

+        if eventLoop.inEventLoop {
+            self.performGroupOperation0(group, interface: interface, promise: promise, operation: .join)
+        } else {
+            eventLoop.execute { self.performGroupOperation0(group, interface: interface, promise: promise, operation: .join) }


weissi · 2018-09-19T14:50:53Z

Sources/NIO/SocketChannel.swift

+        if eventLoop.inEventLoop {
+            self.performGroupOperation0(group, interface: interface, promise: promise, operation: .leave)
+        } else {
+            eventLoop.execute { self.performGroupOperation0(group, interface: interface, promise: promise, operation: .leave) }


weissi · 2018-09-19T14:52:43Z

Sources/NIO/SocketChannel.swift

+                // IPv6 binding with no specific interface requested.
+                let multicastRequest = ipv6_mreq(ipv6mr_multiaddr: groupAddress.address.sin6_addr, ipv6mr_interface: 0)
+                try self.socket.setOption(level: CInt(IPPROTO_IPV6), name: operation.optionName(level: IPPROTO_IPV6), value: multicastRequest)
+            case (.v4, .some(.v6)), (.v6, .some(.v4)), (.v4, .some(.unixDomainSocket)), (.v6, .some(.unixDomainSocket)):


what's the odds that the 4.0 or 4.1 compiler can't see this is exhaustive ;)

weissi · 2018-09-19T14:53:05Z

Sources/NIOMulticastChat/main.swift

+//===----------------------------------------------------------------------===//
+
+import NIO
+


nit: extra newline

weissi · 2018-09-19T14:53:29Z

Sources/NIOMulticastChat/main.swift

+//    guard case .v4(let targetAddress) = targetInterface.address else {
+//        fatalError("Parsed target address as non-IPv4: \(targetInterface.address)")
+//    }
+//    datagramBootstrap = datagramBootstrap.channelOption(ChannelOptions.socket(SocketOptionLevel(IPPROTO_IP), IP_MULTICAST_IF), value: targetAddress.address.sin_addr)


do we want this commented out code?

Lukasa · 2018-09-20T09:08:01Z

Ok, ready for re-review.

weissi

almost there, just one minor naming thing

weissi · 2018-09-20T10:31:34Z

Sources/NIO/SocketOptionChannel.swift

+    ///
+    /// - returns: An `EventLoopFuture` containing the value of the socket option, or
+    ///     any error that occurred while retrieving the socket option.
+    public func getIpMulticastIf() -> EventLoopFuture<in_addr> {


Swift naming conventions say getIPMulticastIF as abbreviations are all caps.

weissi · 2018-09-20T10:31:45Z

Sources/NIO/SocketOptionChannel.swift

+    ///
+    /// - returns: An `EventLoopFuture` containing the value of the socket option, or
+    ///     any error that occurred while retrieving the socket option.
+    public func getIpv6MulticastLoop() -> EventLoopFuture<CUnsignedInt> {


normanmaurer · 2018-09-19T18:57:18Z

Sources/NIO/Channel.swift

+    case unknownLocalAddress
+
+    /// The address family of the multicast group was not valid for this `Channel`.
+    case badMulticastGroupAddressFamily


should this also take the family as parameter (just like we did for notAMulticastAddress) ?

It could, but it would really need both the channel address family and the provided address family to make much sense. Should we do that?

Nah... I think that would be too much :) Ship it !

normanmaurer · 2018-09-19T18:57:29Z

Sources/NIO/Channel.swift

+    case badMulticastGroupAddressFamily
+
+    /// The address family of the provided multicast group join is not valid for this `Channel`.
+    case badInterfaceAddressFamily


Same question as above

normanmaurer · 2018-09-19T18:57:54Z

Sources/NIO/Channel.swift

+
+    /// An attempt was made to join a multicast group that does not correspond to a multicast
+    /// address.
+    case notAMulticastAddress(SocketAddress)


or invalidMulticastAddress

Motivation: A large number of very useful protocols are implemented using multicast with UDP. As a result, it would be helpful to add support for joining and leaving IP multicast groups using SwiftNIO. Modifications: - Defines a MulticastChannel protocol for channels that support joining and leaving multicast groups. - Adds an implementation of MulticastChannel to DatagramChannel. - Adds a interfaceIndex property to NIONetworkInterface. - Adds if_nametoindex to the Posix enum. - Adds a isMulticast computed property to SocketAddress - Adds a demo multicast chat application. - Add a number of multicast-related socket options to SocketOptionProvider. Result: NIO users will be able to write channels that handle multicast UDP.

weissi

thanks very much, I'm happy this!

Lukasa added the 🔼 needs-minor-version-bump For PRs that when merged cause a bump of the minor version, ie. 1.x.0 -> 1.(x+1).0 label Sep 19, 2018

Lukasa added this to the 1.10.0 milestone Sep 19, 2018

Lukasa requested review from normanmaurer and weissi September 19, 2018 14:35

Lukasa force-pushed the cb-udp-multicast branch 2 times, most recently from 8639b33 to d5f9e78 Compare September 19, 2018 14:52

weissi requested changes Sep 19, 2018

View reviewed changes

Lukasa force-pushed the cb-udp-multicast branch 6 times, most recently from e56afef to 57e1a67 Compare September 20, 2018 08:53

weissi requested changes Sep 20, 2018

View reviewed changes

normanmaurer requested changes Sep 20, 2018

View reviewed changes

Lukasa force-pushed the cb-udp-multicast branch from 57e1a67 to b9ece72 Compare September 20, 2018 14:08

weissi approved these changes Sep 21, 2018

View reviewed changes

Merge branch 'master' into cb-udp-multicast

864e4f0

normanmaurer approved these changes Sep 24, 2018

View reviewed changes

Lukasa merged commit c73eb57 into apple:master Sep 24, 2018

Lukasa deleted the cb-udp-multicast branch September 24, 2018 09:24

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Support UDP multicast. #618

Support UDP multicast. #618

Lukasa commented Sep 19, 2018

weissi left a comment

weissi Sep 19, 2018

Lukasa Sep 19, 2018

weissi Sep 19, 2018

normanmaurer Sep 19, 2018

weissi Sep 19, 2018

weissi Sep 19, 2018

weissi Sep 19, 2018

weissi Sep 19, 2018

weissi Sep 19, 2018

weissi Sep 19, 2018

weissi Sep 19, 2018

weissi Sep 19, 2018

Lukasa commented Sep 20, 2018

weissi left a comment

weissi Sep 20, 2018

weissi Sep 20, 2018

normanmaurer Sep 19, 2018

Lukasa Sep 20, 2018

normanmaurer Sep 24, 2018

normanmaurer Sep 19, 2018

normanmaurer Sep 19, 2018

weissi left a comment

		//===----------------------------------------------------------------------===//

		import NIO

Support UDP multicast. #618

Support UDP multicast. #618

Conversation

Lukasa commented Sep 19, 2018

weissi left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Lukasa commented Sep 20, 2018

weissi left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

weissi left a comment

Choose a reason for hiding this comment